[Datepicker] Redesign datepicker as per material spec#3739
[Datepicker] Redesign datepicker as per material spec#3739nathanmarks merged 14 commits intomui:masterfrom
Conversation
045d5bb to
f437272
Compare
|
Hi Nitin, nice work! The landscape picker looks much better already! Just a few style tweaks as promised in gitter 😄 : (1) The date selector overlaps the current selection: In general the vertical spacing looks a bit tight, so increasing that slightly will probably fix this overlap issue too. (2) The days should be vertically aligned with the dates: This left margin may be the issue: (3) There needs to be some margin around the calendar, at least on the sides and bottom, and 8px between the buttons: (4) If we're aiming to follow the spec, it looks like the buttons have a smaller min-width. (Note how the OK button is narrower, even with the extra margin in the MD spec): That may be a fault with our button component, but for now you could override it for the calendar. (5) In 0.15.0-alpha.2, the calendar used to have an animated transition when changing month, similar to: Also in that video, you'll notice the calendar maintains it's height when the number of weeks changes. Currently ours jumps p and down in height. This also affects the year selector. (6) The hover state for the year selector in the redesigned picker should be different to the selected year: Google don't give any hints here, but it should probably be an intermediate state, not the same as the selected year - smaller size than selected, and the lighter green color. IT should probably transition between states. (7) The font size for the unselected years is too small. Redesigned picker: Spec:
Expect more to follow, but that should keep you busy for a little while 😄 |
|
Need to check this for #1489. |
|
@mbrookes : Alright. Will do. |
16c316c to
e3ba816
Compare
@mbrookes: Just added these for you to uncheck if you feel it needs more work. 😀 |
e3ba816 to
9e7a71e
Compare
9e7a71e to
d3dbbec
Compare
|
@tintin1343 - getting there! 👍 I've unchecked the ones that don't appear to have been addressed yet. Also:
It's better, I don't think the currently selected year should shrink when hovered, also could the list be laid out with fixed spacing so it doesn't reposition later years on font size change of earlier ones? |
a8e176b to
9d0d307
Compare
@mbrookes : Please check again. I made the changes you mentioned where not fixed. |
|
Thanks @mbrookes! Those are some amazing observations. I will work on em right away. |
bd1c392 to
b026992
Compare
|
@mbrookes: Updated the changes:
I am not able to reproduce the last one. Can you help me with it? |
b026992 to
bd1c392
Compare
|
Last few issues outstanding.
And the buttons look too tall. Check the spec again (layout and visual).
Still appears to be about 78px: Only if you wanted to try, it would be pretty cool if when you click in a year, it animates to adopt the selected year position, and size. |
83320fa to
0ab730f
Compare
|
|
|
|
@nathanmarks : which browser? Looks fine in chrome, Safari and firefox
They do have. The CalendarActionButtons.js file has this. |
|
@tintin1343 I was on the wrong PR 😄 |
|
@nathanmarks U scared me man. :p |
|
@tintin1343 hahaah I think we should force the month onto the 2nd line here, under all circumstances: usually it is like |
|
@nathanmarks : I agree. Let me fix that. |
| const styles = getStyles(this.props, this.context, this.state); | ||
| const year = selectedDate.getFullYear(); | ||
| const regex = /[.,]/; | ||
| const delimiter = this.props.locale === 'fr' ? '.' : ','; |
There was a problem hiding this comment.
hey dude, this technique isn't going to be scalable... I think we may just have to leave it as it was 😞 there's no way we can account for all the possible locales that a user might pass if they are providing the formatter.
There was a problem hiding this comment.
I know and I agree. This is what I was thinking when I was considering locales. However I don't think this would do any harm as most of the locales will use either a comma or a dot. Only crazy thing would be if there is a hypen or slash.
I can do one thing and it's a crude way/ hack. Leave it as it was append after weekday so that it pushes the day and month to the bottom. What say?
There was a problem hiding this comment.
I meant append.... ..
ebc661d to
8211ddf
Compare
8211ddf to
3b13929
Compare
| shouldDisableDate: PropTypes.func, | ||
| showActionButtons: PropTypes.bool, | ||
| wordings: PropTypes.object, | ||
| wordings: deprecated(PropTypes.object, 'Instead, use `cancelLabel` and `okLabel`.'), |
There was a problem hiding this comment.
You should only need the warning in DatePicker, otherwise the warning will fire multiple times as the value propagates down the component tree.
| shouldDisableDate: PropTypes.func, | ||
| showActionButtons: PropTypes.bool, | ||
| wordings: deprecated(PropTypes.object, 'Instead, use `cancelLabel` and `okLabel`.'), | ||
| wordings: PropTypes.object, |
There was a problem hiding this comment.
Why are we getting rid of a deprecated function without removing the prop?
There was a problem hiding this comment.
@mbrookes asked me to just keep it in the Datepicker and remove em from the child components.
dcb9ded to
7797867
Compare
|
@mbrookes : Fixed the issues that we discussed. |



















This PR deals with the following:
Closes #3603.
Closes #3740.
Closes mui/mui-x#7515.
Closes #1489.
Closes #3046.